Skip to content

[quality] test: pin session-revocation persistence failure semantics (src/pkg/hub/hub_session_revocation.go saveRevokedSessions/loadRevokedSessions) - #6355

Merged
kubestellar-prow[bot] merged 1 commit into
v4from
quality/session-revocation-persistence-tests
Sep 9, 2026
Merged

[quality] test: pin session-revocation persistence failure semantics (src/pkg/hub/hub_session_revocation.go saveRevokedSessions/loadRevokedSessions)#6355
kubestellar-prow[bot] merged 1 commit into
v4from
quality/session-revocation-persistence-tests

Conversation

@kubestellar-hive

@kubestellar-hive kubestellar-hive Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds src/pkg/hub/hub_session_revocation_persistence_test.go pinning the untested failure-path semantics of the hub session-revocation store (src/pkg/hub/hub_session_revocation.go):

  • loadRevokedSessions with an existing-but-unreadable file → fails CLOSED (every sid reads revoked; surfaced via hubSessionRevokedLookup), and a later successful load clears the mode.
  • Corrupt file whose quarantine rename also fails (read-only dir) → still fails closed; the security posture must not depend on the quarantine succeeding.
  • Load-time prune of expired entries rewrites the file without dropping live entries.
  • saveRevokedSessions MkdirAll / WriteFile / Rename failures → logged, non-fatal (it runs on the logout request path), and a failed save leaves the last good file byte-identical and the in-memory revocation intact.
  • Nil-receiver guards on revokedSessions and the HubServer wrappers; input-shape guards and duplicate-revoke semantics of revokeHubSessionCookieAt (with a positive control minting a real v3 cookie).

Coverage: loadRevokedSessions 68.4% → 100%, saveRevokedSessions 57.9% → 89.5%, revokeHubSessionCookieAt 77.3% → 90.9%. Full ./pkg/hub suite green (142s), go vet clean. Permission-based failure injection is skipped under root (same guard as hub_generations_store_test.go).

Claimed ground

Only src/pkg/hub/hub_session_revocation.go persistence/guard functions and the new test file. Disjoint from open quality PRs #6349 (github/prclaims), #6348 (dashboard reissue token), #6343 (dashboard terminal handoff), #6342 (github IssueClosedAt), #6330 (agent thrash poller), #6292 (local mode write roots).

Test plan

  • go test ./pkg/hub/ — all pass
  • go vet ./pkg/hub/ — clean

Filed by quality agent (hold-gated mode). Human review required.

— hive: agent=quality backend=copilot model=claude-fable-5

hive-quality
test: pin session-revocation persistence failure semantics (pkg/hub s…
4976d42 
…aveRevokedSessions/loadRevokedSessions)

The fail-closed and durability error paths of the hub session revocation
store were untested: unreadable-file load (fail closed), corrupt-file
quarantine rename failure, load-time prune rewrite, and the MkdirAll/
WriteFile/Rename failure branches of saveRevokedSessions. These are the
security-critical directions — silently forgetting revocations un-revokes
sessions.

loadRevokedSessions 68.4% -> 100%, saveRevokedSessions 57.9% -> 89.5%,
revokeHubSessionCookieAt 77.3% -> 90.9%.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: hive-quality <sec-check@hive.kubestellar.io>
@kubestellar-hive kubestellar-hive Bot added the hold label Sep 9, 2026
@kubestellar-prow kubestellar-prow Bot added the dco-signoff: yes Indicates the PR's author has signed the DCO. label Sep 9, 2026
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Changelog: this PR changes code but carries no changelog entry
(no changelog.d/ fragment and no CHANGELOG.md edit).

If it is user-visible — a feature, a fix an operator would notice, a
security change, a migration, a deprecation, or anything breaking —
please add a fragment file changelog.d/<category>-<pr-or-slug>.md
containing exactly your entry (see changelog.d/README.md; the
category is one of added/changed/deprecated/fixed/security). Please
do not append to CHANGELOG.md's ## Unreleased directly — that
shared section is the merge-conflict generator fragments replaced
(#5675). If it is a refactor, a test-only change, or dependency
churn, no entry is needed and you can ignore this.

This is a reminder, not a gate; it never blocks a merge.

@kubestellar-prow kubestellar-prow Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 9, 2026
This was referenced Sep 9, 2026
Open
Merged
@clubanderson clubanderson added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 9, 2026
@kubestellar-prow

kubestellar-prow Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubestellar-prow
kubestellar-prow Bot merged commit 511af60 into v4 Sep 9, 2026
54 of 58 checks passed
@kubestellar-prow
kubestellar-prow Bot deleted the quality/session-revocation-persistence-tests branch September 9, 2026 13:46
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Thank you for your contribution! Your PR has been merged.

We'd love to hear how your experience was: share feedback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. copilot dco-signoff: yes Indicates the PR's author has signed the DCO. hold lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@clubanderson